fix: harden schema-validator-helper.sh against set -e premature exit (t142) #449
Conversation
Summary of ChangesHello @marcusquinn, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the robustness and predictability of the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughAdded explicit error handling in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Sat Feb 7 18:01:28 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively hardens the schema-validator-helper.sh script against premature exits when set -e is active. The changes are well-implemented and improve the script's robustness and maintainability. Specifically, adding an error guard for the create_js_script function, documenting the pattern for capturing expected non-zero exit codes, and changing exit 1 to return 1 for consistency are all excellent improvements. The code quality is high, and the changes correctly address the intended fix.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
…y errors (t147.2) (#451) * chore: mark t142 complete in TODO.md (#449) * chore: mark t135.9 complete in TODO.md (#448) * fix(supervisor): triage PR #392 review feedback - stderr, sort, deploy errors (t147.2) - Add post-PR states to ORDER BY CASE in cmd_status and cmd_list - Add 300s timeout to deploy setup.sh with portable fallback - Deploy failures now transition to 'failed' instead of silently marking 'deployed' - Redirect cmd_pr_lifecycle output to post-pr.log instead of /dev/null - Fix missing $SUPERVISOR_DB arg in db() calls for no_pr retry counter - Remove unused no_pr_key variable Dismissed 3 of 6 review threads as already fixed or invalid: - gh pr merge stderr: already uses 2>&1 - Fallback PR lookup: already uses git -C and gh pr list --repo - Missing pr_review:deployed transition: not needed, complete:deployed handles it
…147.1) (#450) * chore: mark t142 complete in TODO.md (#449) * chore: mark t135.9 complete in TODO.md (#448) * fix(supervisor): add missing $SUPERVISOR_DB arg to db() calls, remove HOMEBREW_PREFIX guard (t147.1) - Add $SUPERVISOR_DB as first arg to both db() calls in no_pr retry counter logic (lines 3165, 3183). Without it, sqlite3 treated the SQL as a filename and failed silently — retry counter never persisted, making the 5-attempt threshold unreachable. - Remove unused no_pr_key variable. - Remove HOMEBREW_PREFIX guard around PATH augmentation. The idempotent PATH check already prevents duplicates; the guard was overly restrictive for cron environments where HOMEBREW_PREFIX may be set without all tool paths present. Triaged all 4 unresolved review threads from PR #435: - 3 fixed (critical db bug, unused var, PATH guard) - 1 acknowledged won't-fix (json_extract counter reset is by design) Refs: GH#438, t147.1
marcusquinn
force-pushed
the
feature/t142
branch
from
433de09 to
9aeaec9
Compare
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Sat Feb 7 19:01:04 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
…(t142) - Add error guard on create_js_script call in cmd_validate - Document the || exit_code=$? pattern that prevents set -e from killing the script before validation results are reported - Change exit 1 to return 1 in main() catch-all for consistency The core || exit_code=$? guard was already in place from PR #391. This commit adds defensive improvements for remaining set -e hazards. Closes #443
marcusquinn
force-pushed
the
feature/t142
branch
from
9aeaec9 to
57aa741
Compare
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Sat Feb 7 19:08:20 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
1 participant
Summary
create_js_scriptcall incmd_validateto prevent silentset -eexit|| exit_code=$?pattern that preventsset -efrom killing the script before validation results are reportedexit 1toreturn 1inmain()catch-all for consistency with the rest of the scriptAnalysis
The core
|| exit_code=$?guard on the node subshell was already in place from PR #391. The CodeRabbit review flagged this as a critical issue, but the code was already correct for the primary validation path. This PR adds defensive improvements for the remainingset -ehazards:create_js_scriptunguarded call — ifmkdir -porcat >fails,set -ewould kill the script without a useful error message. Now wrapped with|| { print_error ...; return 1; }exit 1in catch-all — usedexitinstead ofreturn, which is inconsistent and prevents callers from handling the error. Changed toreturn 1|| exit_code=$?pattern is necessaryTesting
bash -n: syntax check passes|| exit_code=$?pattern correctly captures non-zero exit codes underset -euo pipefailCloses #443
Summary by CodeRabbit
Bug Fixes
Documentation
Chores